Add tests for flowable engine #1720
Conversation
JIRA:LMCROSSITXSADEPLOY-3320
...src/test/java/org/cloudfoundry/multiapps/controller/process/flowable/FlowableEngineTest.java
Outdated
Show resolved
Hide resolved
...src/test/java/org/cloudfoundry/multiapps/controller/process/flowable/FlowableEngineTest.java
Outdated
Show resolved
Hide resolved
| configuration.setDefaultMailClient(flowableMailClient); | ||
| configuration.setDatabaseSchemaUpdate("create-drop"); | ||
| configuration.setBeans(Map.of("testVariableTask", testVariableTask)); | ||
| configuration.setAsyncExecutorActivate(false); |
There was a problem hiding this comment.
why is the async executor turned off?
There was a problem hiding this comment.
It was generated by the chat in order to keep the test single threaded but even when this line is removed, the test is still single thread. So we can remove this configuration.
...src/test/java/org/cloudfoundry/multiapps/controller/process/flowable/FlowableEngineTest.java
Outdated
Show resolved
Hide resolved
...src/test/java/org/cloudfoundry/multiapps/controller/process/flowable/FlowableEngineTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Test | ||
| void testProcessWithPredefinedVariables() { | ||
| String correlationId = UUID.randomUUID() |
There was a problem hiding this comment.
A lot of boilerplate for building test data (createBigDeploymentDescriptor, etc.) makes the class huge.
Suggestion: extract TestFixtures (factory classes or builders) into a separate test utility
...src/test/java/org/cloudfoundry/multiapps/controller/process/flowable/FlowableEngineTest.java
Show resolved
Hide resolved
| // This assertion intentionally checks that the modified deployment descriptor is different from the initially set one because of flowable caching regression | ||
| // https://github.com/flowable/flowable-engine/issues/4130 | ||
| // The original behaviour must be to have same deployment descriptor as it was set in the task. Modify the test to assert for matching values when the issue is fixed. | ||
| assertNotEquals(JsonUtil.toJson(modifiedBigDeploymentDescriptor), JsonUtil.toJson(retrievedBigDeploymentDescriptor)); |
There was a problem hiding this comment.
This is odd, why do we need to create a test case to check a bug/regression, do you think that we can add it with assertEquals and comment it until a fix is provided?
There was a problem hiding this comment.
This will remind us to fix the test when new version of flowable is adopted and fix is available and also remove custom added workaround in FlowableConfiguration class.
| Map<String, Object> initialVariables = new HashMap<>(); | ||
| setSerializedValueInMap(initialVariables, Variables.CORRELATION_ID, correlationId); | ||
| setSerializedValueInMap(initialVariables, Variables.DELETE_SERVICES, deleteServices); | ||
| setSerializedValueInMap(initialVariables, Variables.MTA_ID, mtaId); |
There was a problem hiding this comment.
The core logic of the tests is ~20 lines you may think of extracting the assertions and the setup in separate utilities for more readable tests.
|
3 participants
No description provided.